feat(intro): Add short Intro Logo for The Super Hackers team#2267
feat(intro): Add short Intro Logo for The Super Hackers team#2267xezon wants to merge 10 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/GameClient/Intro.cpp | Adds the intro state machine, logo screen, timing, and fade behavior. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp | Creates, updates, renders, completes, and destroys the intro controller. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Calls the GameClient draw hook from the W3D render sequence. |
Reviews (15): Last reviewed commit: "fixup! refactor(intro): Move intro relat..." | Re-trigger Greptile
Additional Comments (1)
Fix by making Prompt To Fix With AIThis is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp
Line: 514:531
Comment:
**Intro can block startup**
`GameClient::update()` only shows the shell (`TheShell->showShellMap/showShell`) after `m_intro->isDone()`. With the new `Intro` implementation, if the global flags disable both intro and sizzle (`m_playIntro==false` and `m_playSizzle==false`) and `m_afterIntro` is not pre-set, `Intro` never reaches `Done` (see `IntroState_Start` handling), so the game can get stuck in the intro phase and never show the shell. This previously worked because `GameEngine::init()` forced `m_afterIntro = TRUE` when `m_playIntro` was false.
Fix by making `Intro` immediately become done when there are no enabled states, or by preserving the old `m_afterIntro` initialization behavior.
How can I resolve this? If you propose a fix, please make it concise. |
999721b to
0f278e6
Compare
0f278e6 to
1a738f7
Compare
|
Wouldn't it be more appropriate to show the Superhackers stuff in the credits menu? As is this would imply that the whole game is made by us. I don't think it would be fair to show our names more prominently than the original developers. |
Maybe remove all user names from this Intro and just show the Team name + url? We can also clarify "Edited by ..." |
I wouldn't add anything to the intro. I don't think it's necessary to remind the player every time the game starts who made the patch. Maybe just add a tiny note on the shellmap screen below the gamename "patched by Superhackers" or something like that. It's nice though that the 3-5 seconds black screen is removed here! |
|
I didn't even know what the 'black screen' was when I saw this PR, because I have renamed the files for the EA logo and sizzle and I don't see it. This makes the game launch faster, and saves me 5+ seconds every time I launch the game. I strongly dislike changing anything that would unnecessarily increase the launch time for me as developer. I'm ok with it if I can skip it with some command line. That said, I like the new intro. |
|
I don't think people will understand what it means if the intro screen is this plain. On the intro, maybe add 3 to 5 sentences with the key achievements of TheSuperHackers. Maybe something like: |
I feel like adding too much text won't really be very useful given the fact that it's only shown for 3 seconds. |
I agree with this sentiment. What we can add is a "Improved by" on top, to clarify that this Author has improved the game. This is very simple and broad. |
|
When this is merged I will make a follow up to skip the entire intro sequence with a single ESC press. This will alleviate any waiting pains for impatient users. |
1a738f7 to
29b1ce7
Compare
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp
Line: 119:119
Comment:
Missing cleanup of `m_intro` in destructor - potential memory leak if `GameClient` is destroyed before intro completes.
```suggestion
delete m_intro;
m_intro = nullptr;
```
How can I resolve this? If you propose a fix, please make it concise. |
29b1ce7 to
9c82551
Compare
|
Maybe text can be made a bit bigger. |
|
A suggestion: the credits screen would display all contributors names. Something like: The |
|
Needs rebase and review. |
|
|
||
| if (TheGlobalData->m_playSizzle) | ||
| { | ||
| if (TheGlobalData->m_playIntro) |
There was a problem hiding this comment.
nit: style, always use braces, several times in this file: line 55, 67, 102, 219, 274
There was a problem hiding this comment.
For small conditions it is generally ok when it reads ok.
|
Oh wow why does the ASCII text fall apart like this? |
|
I left some nits, and it needs another rebase, but it looks good to me. I think ESC should work to skip, and/or a -quickstart arg would be nice. |
Oof that's no good. I'm in favor of using an image instead of ascii. If we need to troubleshoot the ascii, claude says "pick a much larger sizeY (so GDI's full-block glyph actually fills its cell), guarantee the font is installed (or use one shipped with the game), and stop the setWordWrap call from running every draw" - but I still think an image is easier and safer |
bf25fd5 to
2679b54
Compare
I changed the font. Please try again. A mono font should work. That said, the previous font was supposed to be mono, but maybe it is not on all systems.
www. removed.
There is I tried triggering it but that did not work, probably because it is a positional sound. Would require more effort to get working.
I expect that many players could perceive this as "gay wording", because "lovingly" does not fit the theme of the game. |
…movie code (#2267)
2679b54 to
7702190
Compare
7702190 to
5e10a09
Compare
| , sizeY(10) | ||
| , centerOffsetY(0) | ||
| {} | ||
| ~DisplayEntity(); |
There was a problem hiding this comment.
DisplayEntity frees displayString in its destructor but has implicit copy ctor/assignment and lives in a std::vector, maybe free the strings in a ~Intro loop instead? I think this would only be an issue if someone appends a second credits screen though
There was a problem hiding this comment.
Yes that is not so elegant. It would be solved by making the raw pointer a unique_ptr with c++11. I added a comment for now.
|
Looks beautiful! I found that it crashes if I exit/alt f4 during our credit screen, left a couple of comments. |







Merge with Rebase
This change has 5 commits:
Old Intro Sequence
(*1) I did not measure how long the black screen was. According to code somewhere between 3 to 5 seconds.
New Intro Sequence
TODO